-
-
Notifications
You must be signed in to change notification settings - Fork 2k
allow arbitrary types in set difference #15160
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This comment has been minimized.
This comment has been minimized.
db50008 to
57c542c
Compare
This comment has been minimized.
This comment has been minimized.
|
Diff from mypy_primer, showing the effect of this PR on open source code: pandas (https://github.com/pandas-dev/pandas)
+ pandas/core/dtypes/cast.py:869: error: Unused "type: ignore" comment [unused-ignore]
+ pandas/core/dtypes/cast.py:870: error: Unused "type: ignore" comment [unused-ignore]
ibis (https://github.com/ibis-project/ibis)
- ibis/backends/sql/datatypes.py:1398: error: Unsupported operand types for - ("set[type[Never]]" and "set[type[SqlglotType]]") [operator]
- ibis/expr/datatypes/tests/test_core.py:581: error: Unsupported operand types for - ("set[type[Never]]" and "set[AnnotableMeta]") [operator]
schemathesis (https://github.com/schemathesis/schemathesis)
- src/schemathesis/config/_error.py:54: error: Argument 1 to "set" has incompatible type "Any | Unset"; expected "Iterable[Any | None]" [arg-type]
+ src/schemathesis/config/_error.py:54: error: Argument 1 to "set" has incompatible type "Any | Unset"; expected "Iterable[object]" [arg-type]
pydantic (https://github.com/pydantic/pydantic)
- pydantic/v1/main.py:924: error: Set comprehension has incompatible type Set[int | str]; expected Set[str | None] [misc]
- pydantic/v1/main.py:924: note: Left operand is of type "set[str] | dict_keys[str, Any]"
|
srittau
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm generally not a big fan of allowing object in cases like this, but the primer output (and consistency with __isub__) speaks for itself. Also, {1,2,3} - {1,2,"ab"} is a valid use case.
|
@srittau Maybe we should be using |
|
I'd also like to get @AlexWaygood's opinion, as you discussed changing |
|
This seems like a good change to me. I agree that when typeshed is too opinionated, it often causes more problems than it solves; we've been running into a few pain points on the ty issue tracker recently with similar causes. Please let's not (ever) change the parameter type to |
|
Yeah, that whole hashability thing is a big mess. I also recently learned one can make unhashable classes via custom metaclasses, which is concerning. Anyway, so is this good, or are there any other reasons to prefer |
|
I think either is fine; it doesn't really make much difference for parameter annotations. We sometimes use |
|
Thanks @randolf-scholz! |
Fixes #9004
See also: #11403
This PR allows set differences with disjoint generic types. This option was previously argued against (#9004 (comment)) because
However,
typeshedshouldn't try to act as a linter. That's the job of the type checker. There are cases when the current setup hurts, for instance when we try to doset[Literal["a", "b"]] - set[str].This should be handled on the type-checker side, for instance by warning on
set[T] - set[S]ifTandShave empty intersection (⇝ disjoint_bases https://peps.python.org/pep-0800/), which would catch the bad case described above.